Skip to content

Conversation

@vweevers
Copy link
Member

@vweevers vweevers commented Jul 2, 2022

Combines #6 and #1. I've forked length-prefixed-stream to make that use readable-stream v4 as well.

Semver-major because the streams exposed here now don't emit duplexify's custom events (prefinish, preend, cork, uncork) although it's unlikely that anyone is using those events (or even many-level itself, which is relatively new).

One thing left to do: test against rave-level (which also depends on readable-stream and failed a test: Level/rave-level#3).

cc @ronag

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@vweevers vweevers added the semver-major Changes that break backward compatibility label Jul 2, 2022
Copy link
Contributor

@ronag ronag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woho!

@vweevers
Copy link
Member Author

vweevers commented Jul 2, 2022

One thing left to do: test against rave-level (which also depends on readable-stream and failed a test: Level/rave-level#3).

That test failure is unrelated to many-level. See nodejs/readable-stream#477

@vweevers
Copy link
Member Author

vweevers commented Jul 2, 2022

I managed to work around that, but then another issue popped up. Don't want to spend more time on that, so I'm just gonna drop support of Node.js 12 in rave-level.

In any case, this PR is good to go.

@vweevers vweevers merged commit 308bde7 into main Jul 2, 2022
@vweevers vweevers deleted the readable-stream-v4 branch July 2, 2022 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-major Changes that break backward compatibility

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants